Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Optimize manifest directory lookup with efficient caching #6444

Merged
merged 4 commits into from
Aug 22, 2024
Merged

Optimize manifest directory lookup with efficient caching #6444

merged 4 commits into from
Aug 22, 2024

Conversation

JoshuaBatty
Copy link
Member

@JoshuaBatty JoshuaBatty commented Aug 21, 2024 •
edited
Loading

Description

Implemented a caching mechanism for manifest directory lookups using a DashMap. This optimization reduces average lookup time from approximately 421µs to 0.47µs, resulting in an 895x speed improvement for subsequent calls.

part of #5445

Checklist

  • I have linked to any relevant issues.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have updated the documentation where relevant (API docs, the reference, and the Sway book).
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added (or requested a maintainer to add) the necessary Breaking* or New Feature labels where relevant.
  • I have done my best to ensure that my PR adheres to the Fuel Labs Code Review Standards.
  • I have requested a review from the relevant team or maintainers.

@JoshuaBatty JoshuaBatty requested a review from a team as a code owner August 21, 2024 01:52
@JoshuaBatty JoshuaBatty self-assigned this Aug 21, 2024
@JoshuaBatty JoshuaBatty added language server LSP server performance Everything related to performance, speed wise or memory wise. labels Aug 21, 2024
@JoshuaBatty JoshuaBatty mentioned this pull request Aug 21, 2024
Open
12 tasks
@github-actions GitHub Actions
Copy link

Benchmark for e5f2e4b

Click to view benchmark
Test Base PR %
code_action 5.1±0.07ms 5.1±0.07ms 0.00%
code_lens 300.4±7.89ns 300.4±8.24ns 0.00%
compile 2.7±0.03s 2.7±0.04s 0.00%
completion 4.6±0.05ms 4.6±0.04ms 0.00%
did_change_with_caching 2.6±0.04s 2.6±0.05s 0.00%
document_symbol 897.6±74.00µs 911.3±47.17µs +1.53%
format 68.2±0.65ms 68.2±0.84ms 0.00%
goto_definition 341.1±3.76µs 345.7±6.98µs +1.35%
highlight 8.7±0.01ms 8.7±0.13ms 0.00%
hover 495.9±2.70µs 496.2±6.06µs +0.06%
idents_at_position 119.0±0.31µs 122.5±1.35µs +2.94%
inlay_hints 634.6±9.24µs 636.2±28.44µs +0.25%
on_enter 2.1±0.04µs 2.0±0.07µs -4.76%
parent_decl_at_position 3.6±0.03ms 3.6±0.04ms 0.00%
prepare_rename 341.7±5.73µs 349.1±10.41µs +2.17%
rename 9.0±0.10ms 9.0±0.13ms 0.00%
semantic_tokens 1234.5±10.77µs 1228.6±10.17µs -0.48%
token_at_position 341.1±2.51µs 336.5±3.20µs -1.35%
tokens_at_position 3.6±0.04ms 3.6±0.01ms 0.00%
tokens_for_file 401.8±3.00µs 402.0±3.32µs +0.05%
traverse 34.2±1.12ms 34.4±0.83ms +0.58%

sdankel
sdankel approved these changes Aug 22, 2024
View reviewed changes
Copy link
Member

@sdankel sdankel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. There was a failing test though.

@github-actions GitHub Actions
Copy link

Benchmark for abd25d0

Click to view benchmark
Test Base PR %
code_action 5.0±0.01ms 5.1±0.08ms +2.00%
code_lens 301.3±6.24ns 299.2±11.75ns -0.70%
compile 2.6±0.04s 2.6±0.06s 0.00%
completion 4.5±0.01ms 4.5±0.07ms 0.00%
did_change_with_caching 2.6±0.05s 2.5±0.06s -3.85%
document_symbol 971.0±34.50µs 1032.1±26.63µs +6.29%
format 67.3±0.58ms 67.7±0.94ms +0.59%
goto_definition 345.7±5.45µs 344.4±7.10µs -0.38%
highlight 8.7±0.01ms 8.7±0.02ms 0.00%
hover 498.5±8.03µs 499.1±7.53µs +0.12%
idents_at_position 117.1±0.29µs 120.2±0.89µs +2.65%
inlay_hints 633.2±7.52µs 626.3±20.19µs -1.09%
on_enter 2.2±0.14µs 1959.8±79.80ns -10.92%
parent_decl_at_position 3.6±0.03ms 3.6±0.04ms 0.00%
prepare_rename 338.0±6.00µs 343.2±9.99µs +1.54%
rename 9.0±0.09ms 9.0±0.21ms 0.00%
semantic_tokens 1253.4±42.25µs 1224.2±41.28µs -2.33%
token_at_position 339.5±2.32µs 331.2±2.55µs -2.44%
tokens_at_position 3.5±0.03ms 3.6±0.14ms +2.86%
tokens_for_file 403.7±1.63µs 406.2±3.29µs +0.62%
traverse 34.1±0.85ms 34.1±0.58ms 0.00%

@JoshuaBatty JoshuaBatty requested a review from a team August 22, 2024 00:33
@JoshuaBatty
Copy link
Member Author

LGTM. There was a failing test though.

Thanks, this test should be fixed once #6446 is merged.

@JoshuaBatty JoshuaBatty requested a review from a team August 22, 2024 10:37
@JoshuaBatty JoshuaBatty enabled auto-merge (squash) August 22, 2024 10:37
@github-actions GitHub Actions
Copy link

Benchmark for 43ad728

Click to view benchmark
Test Base PR %
code_action 5.1±0.07ms 5.4±0.20ms +5.88%
code_lens 307.0±30.77ns 291.1±7.87ns -5.18%
compile 2.7±0.06s 2.7±0.04s 0.00%
completion 4.5±0.08ms 4.5±0.01ms 0.00%
did_change_with_caching 2.6±0.06s 2.6±0.03s 0.00%
document_symbol 922.8±32.50µs 916.1±16.74µs -0.73%
format 69.3±3.28ms 68.3±0.74ms -1.44%
goto_definition 337.0±6.95µs 345.2±5.85µs +2.43%
highlight 8.7±0.10ms 8.8±0.46ms +1.15%
hover 497.8±12.47µs 494.6±8.47µs -0.64%
idents_at_position 117.3±1.01µs 120.8±1.60µs +2.98%
inlay_hints 626.6±19.12µs 634.8±35.33µs +1.31%
on_enter 2.1±0.02µs 1947.3±104.02ns -7.27%
parent_decl_at_position 3.6±0.04ms 3.6±0.03ms 0.00%
prepare_rename 338.9±18.36µs 357.4±10.10µs +5.46%
rename 9.0±0.18ms 9.5±0.39ms +5.56%
semantic_tokens 1170.3±22.81µs 1218.6±10.10µs +4.13%
token_at_position 335.1±3.54µs 339.4±27.91µs +1.28%
tokens_at_position 3.6±0.08ms 3.6±0.05ms 0.00%
tokens_for_file 401.8±4.08µs 403.9±1.62µs +0.52%
traverse 34.6±0.54ms 35.1±1.43ms +1.45%

@github-actions GitHub Actions
Copy link

Benchmark for e0449de

Click to view benchmark
Test Base PR %
code_action 5.1±0.01ms 5.1±0.08ms 0.00%
code_lens 288.6±14.89ns 286.3±7.85ns -0.80%
compile 2.7±0.07s 2.7±0.06s 0.00%
completion 4.5±0.01ms 4.5±0.07ms 0.00%
did_change_with_caching 2.6±0.05s 2.6±0.04s 0.00%
document_symbol 923.9±41.53µs 923.3±39.64µs -0.06%
format 67.9±0.84ms 69.2±0.82ms +1.91%
goto_definition 349.6±6.51µs 340.9±6.21µs -2.49%
highlight 8.8±0.48ms 8.8±0.14ms 0.00%
hover 505.7±5.25µs 495.4±19.20µs -2.04%
idents_at_position 117.9±1.94µs 122.2±1.47µs +3.65%
inlay_hints 639.2±8.76µs 633.2±23.13µs -0.94%
on_enter 2.0±0.08µs 2.0±0.05µs 0.00%
parent_decl_at_position 3.6±0.07ms 3.6±0.02ms 0.00%
prepare_rename 345.7±3.94µs 344.1±6.98µs -0.46%
rename 9.0±0.15ms 9.0±0.15ms 0.00%
semantic_tokens 1244.8±9.01µs 1236.3±60.25µs -0.68%
token_at_position 338.4±3.04µs 345.7±2.38µs +2.16%
tokens_at_position 3.6±0.05ms 3.6±0.07ms 0.00%
tokens_for_file 410.9±5.32µs 400.7±2.96µs -2.48%
traverse 35.0±0.82ms 34.7±0.72ms -0.86%

@JoshuaBatty JoshuaBatty merged commit 5840e8d into master Aug 22, 2024
39 checks passed
@JoshuaBatty JoshuaBatty deleted the josh/manifest_cache branch August 22, 2024 19:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kayagokalp kayagokalp kayagokalp approved these changes

@sdankel sdankel sdankel approved these changes

Assignees

@JoshuaBatty JoshuaBatty

Labels
language server LSP server performance Everything related to performance, speed wise or memory wise.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@JoshuaBatty @kayagokalp @sdankel @IGI-111